Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use state to update a Command's input #456

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ChickenProp
Copy link
Contributor

This allows you to write commands whose input changes when a previous
Command shrinks.

For example, suppose the state contains someList :: [Bool] and
you have a command whose input expects "index into someList pointing
at True". You can generate that index directly, but if an earlier
command shrinks, someList might change and your index now points at
False.

(This is contrived, but hopefully points at the sorts of more
complicated situations where it might be useful.)

With this you can instead generate a number between 0 and (the number of
True elements). Then use mkInput to turn that number into an index
into someList. This will still be valid as long as the number of
True elements doesn't shrink below the generated value.

You could also pass this number directly into exec. But then in exec
you'd need to get someList directly from the concrete model, which
might be complicated and/or slow.

I implemented this by adding a new Command constructor, CommandA
where A is for Advanced. I don't love this. I could have simply changed
the existing constructor, but that means every existing Command needs to
be updated. (It's a simple change, commandMkInput = const Just means
they work as before, but still a massive pain.) The downside of this
approach is implementation complexity, plus any user functions taking a
Command as input may need to be updated.

Other approaches we could take here:

  1. We could pass the concrete state into exec along with the concrete
    input. Then you wouldn't need to get someList from the model. But you
    might still need to do complicated calculations in exec which could
    make the failure output hard to follow.

    If we did this we'd have the same tradeoff between changing the
    existing constructor and adding a new one.

  2. We could add a callback

    MapMaybeInput
      (state Symbolic -> input Symbolic -> Maybe (input Symbolic)

    Each of these would be applied in turn to update the input.
    (Require would be equivalent to a MapMaybeInput that ignores
    state, and either returns the input unchanged or Nothing.)

    This would be compatible with existing commands, but functions
    accepting a Command or a Callback as input might still need
    changing.

This PR includes #453, and was merged into my own fork as
ChickenProp#3.

@jacobstanley jacobstanley force-pushed the master branch 3 times, most recently from 4139585 to c228279 Compare May 22, 2022 14:42
@jacobstanley
Copy link
Member

I'm focusing on the other PR at the moment but I did have a thought about this one.

Maybe we can do something with PatternSynonyms so that pattern Command constructs a CommandA?

@ChickenProp
Copy link
Contributor Author

Looking at the docs... I might be missing something, but I don't think so, unfortunately.

What I think we want is for a way to use the Command {...} pattern in an expression context. It sounds like there are two difficulties. The first is that we'd also need to be able to use it in a pattern context, and I don't think we can. There's no way to check whether a CommandA is a valid Command. We might be able to hack around this with a pattern that never matches; I haven't tried, but something like

pattern Command {commandGen, commandExecute, commandCallbacks} <-
  CommandA commandGen _ commandExecute commandCallbacks | False

might be valid? I'm not sure. The existential type variables might cause problems.

The other is that record patterns aren't allowed to be explicitly bidirectional. ("Because the rhs expr might be constructing different data constructors", though I don't really know what the docs mean by that.) So even if we could get past the first problem, we wouldn't be able to use record syntax. It would basically just be a smart constructor that starts with a capital letter.

@ChickenProp
Copy link
Contributor Author

ChickenProp commented Jun 6, 2022

Though, another possibility: add a ToAction class and have Command and CommandA be different data types.

Presumably the class method would either replace the existing action, i.e.

class ToAction cmd where
  action ::
       (MonadGen gen, MonadTest m)
    => [cmd gen m state]
    -> GenT (StateT (Context state) (GenBase gen)) (Action m state)

or the class method would only take a single cmd gen m state as input and produce... something, I'm not sure yet.

This wouldn't be fully backwards compatible, in that sequential range state [] will currently compile but it wouldn't if sequential is polymorphic over ToAction. But I think it might be backwards compatible in terms of anything that would ever be useful?

It would make users choose "either all commands are simple or all are complex", but toCommandA :: Command gen m state -> CommandA gen m state is really easy, so that's not a big deal I think.

This allows you to write commands whose input changes when a previous
Command shrinks.

For example, suppose the state contains `someList :: [Bool]` and
you have a command whose input expects "index into `someList` pointing
at `True`". You can generate that index directly, but if an earlier
command shrinks, `someList` might change and your index now points at
`False`.

(This is contrived, but hopefully points at the sorts of more
complicated situations where it might be useful.)

With this you can instead generate a number between 0 and (the number of
`True` elements). Then use `mkInput` to turn that number into an index
into `someList`. This will still be valid as long as the number of
`True` elements doesn't shrink below the generated value.

You could also pass this number directly into `exec`. But then in `exec`
you'd need to get `someList` directly from the concrete model, which
might be complicated and/or slow.

I implemented this by adding a new `Command` constructor, `CommandA`
where A is for Advanced. I don't love this. I could have simply changed
the existing constructor, but that means every existing Command needs to
be updated. (It's a simple change, `commandMkInput = const Just` means
they work as before, but still a massive pain.) The downside of this
approach is implementation complexity, plus any user functions taking a
`Command` as input may need to be updated.

Other approaches we could take here:

1. We could pass the concrete state into `exec` along with the concrete
input. Then you wouldn't need to get `someList` from the model. But you
might still need to do complicated calculations in `exec` which could
make the failure output hard to follow.

     If we did this we'd have the same tradeoff between changing the
     existing constructor and adding a new one.

2. We could add a callback

    ```haskell
    MapMaybeInput
      (state Symbolic -> input Symbolic -> Maybe (input Symbolic)
    ```

    Each of these would be applied in turn to update the input.
    (`Require` would be equivalent to a `MapMaybeInput` that ignores
    state, and either returns the input unchanged or `Nothing`.)

    This would be compatible with existing commands, but functions
    accepting a `Command` or a `Callback` as input might still need
    changing.
We don't need `actionInput0`, because it was only used to pass to
`actionRefreshInput`. So we can just hide it in a closure.
@ChickenProp ChickenProp force-pushed the command-mkInput-upstream branch from 9bee2a8 to 485cbb5 Compare December 2, 2022 10:11
@ChickenProp
Copy link
Contributor Author

@jacobstanley Gentle ping - it would be great if you could take a look at this when you get a chance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants